-
Notifications
You must be signed in to change notification settings - Fork 1.9k
"borrow checker invariants" section of the "leveraging the type system" chapter #2867
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
"borrow checker invariants" section of the "leveraging the type system" chapter #2867
Conversation
…n to lifetime connections
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I apologize for the enormous delay here -- it's been a busy couple of months!
This looks great, and I appreciate seeing previous comments included here.
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass, generally looking good, but I left some comments that may lead to structural changes, so I expect to do another detailed pass after you land some changes.
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,80 @@ | |||
| --- | |||
| minutes: 0 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the timing information.
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
Outdated
Show resolved
Hide resolved
|
|
||
| - By keeping constructors private and not implementing clone/copy for a type, | ||
| making the interior type opaque (as per the newtype pattern), we can prevent | ||
| multiple uses of the same, API-controlled value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not clear what "API-controlled" means here.
| do_something_with_transaction(transaction); | ||
| let assumed_the_transactions_happened_immediately = db.results(); // ❌🔨 | ||
| do_something_with_transaction(transaction); | ||
| // Works, as the lifetime of "transaction" as a reference ended above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be misunderstanding your intent behind this example, but it does not feel realistic realistic because the database didn't get a notification that the borrow has ended, and thus could not have started executing it. In other words, it is not clear what the DatabaseConnection type achieved by being locked out while the transaction was borrowed out.
Maybe you can fix it by wrapping the reference in a custom wrapper type which implements Drop? Ideally, that drop() would also print a message (like "executing query...") to make it clear to the audience that the database can indeed fill in the result list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not done yet.
I previously left a comment here:
I might be misunderstanding your intent behind this example, but it does not feel realistic realistic because the database didn't get a notification that the borrow has ended, and thus could not have started executing it. In other words, it is not clear what the DatabaseConnection type achieved by being locked out while the transaction was borrowed out.
Maybe you can fix it by wrapping the reference in a custom wrapper type which implements Drop? Ideally, that drop() would also print a message (like "executing query...") to make it clear to the audience that the database can indeed fill in the result list.
Here's my version of the example that incorporates the suggestion in a different way, without Drop (hopefully even simpler and shorter):
pub struct QueryResult;
pub struct DatabaseConnection { /* fields omitted */ }
impl DatabaseConnection {
pub fn new() -> Self { Self {} }
pub fn results(&self) -> &[QueryResult] {
&[] // fake results
}
}
pub struct Transaction<'a> {
connection: &'a mut DatabaseConnection,
}
impl<'a> Transaction<'a> {
pub fn new(connection: &'a mut DatabaseConnection) -> Self {
Self { connection }
}
pub fn query(&mut self, _query: &str) {
// Send the query over, but don't wait for results.
}
pub fn commit(self) {
// Finish executing the transaction and retrieve the results.
}
}
fn main() {
let mut db = DatabaseConnection::new();
// The transaction `tx` mutably borrows `db`.
let mut tx = Transaction::new(&mut db);
tx.query("SELECT * FROM users");
// This won't compile because `db` is already mutably borrowed.
let results = db.results(); // ❌🔨
// The borrow of `db` ends when `tx` is consumed by `commit`.
tx.commit();
// Now it is possible to borrow `db` again.
let results = db.results();
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for not elaborating earlier: The drop implementation detail suggestion was too far into the weeds for what I was aiming to accomplish with this slide. This second suggestion is a lot more in line with what I was considering, I'll bring that in + make the narrative more clear in the speaker notes.
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Outdated
Show resolved
Hide resolved
| majority of them. See: | ||
| [Lifetime Elision](../../../lifetimes/lifetime-elision.md). | ||
|
|
||
| </details> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This slide might be a place where an analogy might help some listeners understand what we are trying to get at when we say that the borrow checker and lifetimes are just another API design tool. Here's one possible analogy. WDYT?
Generics in Java were added primarily to support type-safe collections. In fact, Java 5 added generic type arguments to existing standard library collection types that were previously non-generic! So the language designers had a clear primary use case in mind. However, generics turned out to be useful in many other API designs. So it would be too narrow-minded to present Java generics as "a language feature for type-safe collections."
Similarly, the lifetimes and the borrow checker were introduced in Rust for compile-time memory safety guarantees, but their applicability in API design is broader. We (the Rust community) are still discovering design patterns and trying to understand what these tools can do for API design beyond memory safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a good thing to bring up, I'll pull together some references to drop in. If you've got suggestions on pieces covering this I'd be happy to hear about them, but I understand linkrot and the ephemeral nature of back-channel discussion of the time may have gotten to most of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSR 14 which introduced generics lists only one goal specific to a use case, and it is "Good collections support. The core Collections APIs and similar APIs are perhaps the most important customers of genericity, so it is essential that they work well as, and with, generic classes." Furthermore, this is the #1 goal of the proposal overall.
An empirical research article that I could find, Java generics adoption: how new features are introduced, championed, or ignored studies how generics were adopted in practice. It includes a data-driven argument that the most common parameterized types are collections (the only non-collection-related type in Table 1 is Class<?>). This aligns with my intuition: the primary use case is collections, but there are other cases where generics turned out to be useful (for example, Class<?> in the standard library, TypeToken<?> in Google's Guava to work around type erasure in Java's generics, or the "recursive generics" pattern similar to CRTP in C++).
One source that I had high hopes for, ACM's History of programming languages journal, unfortunately does have a piece on Java.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderful, thank you!
Co-authored-by: Dmitri Gribenko <[email protected]>
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Dmitri Gribenko <[email protected]>
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
Outdated
Show resolved
Hide resolved
|
|
||
| - By tying nonce creation and consumption up in rust's ownership model, and by | ||
| not implementing clone/copy on sensitive single-use data, we can prevent | ||
| this kind of dangerous misuse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not done? I previously left a comment here:
I think this bullet point is overlapping with "By keeping constructors private" above. Please consolidate.
|
|
||
| Since then, users and developers of the language expanded the use of generics | ||
| to other areas of type-safe API design. | ||
| <!-- TODO: Reference how this was adopted --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unresolved TODO.
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
Outdated
Show resolved
Hide resolved
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
Outdated
Show resolved
Hide resolved
| do_something_with_transaction(transaction); | ||
| let assumed_the_transactions_happened_immediately = db.results(); // ❌🔨 | ||
| do_something_with_transaction(transaction); | ||
| // Works, as the lifetime of "transaction" as a reference ended above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not done yet.
I previously left a comment here:
I might be misunderstanding your intent behind this example, but it does not feel realistic realistic because the database didn't get a notification that the borrow has ended, and thus could not have started executing it. In other words, it is not clear what the DatabaseConnection type achieved by being locked out while the transaction was borrowed out.
Maybe you can fix it by wrapping the reference in a custom wrapper type which implements Drop? Ideally, that drop() would also print a message (like "executing query...") to make it clear to the audience that the database can indeed fill in the result list.
Here's my version of the example that incorporates the suggestion in a different way, without Drop (hopefully even simpler and shorter):
pub struct QueryResult;
pub struct DatabaseConnection { /* fields omitted */ }
impl DatabaseConnection {
pub fn new() -> Self { Self {} }
pub fn results(&self) -> &[QueryResult] {
&[] // fake results
}
}
pub struct Transaction<'a> {
connection: &'a mut DatabaseConnection,
}
impl<'a> Transaction<'a> {
pub fn new(connection: &'a mut DatabaseConnection) -> Self {
Self { connection }
}
pub fn query(&mut self, _query: &str) {
// Send the query over, but don't wait for results.
}
pub fn commit(self) {
// Finish executing the transaction and retrieve the results.
}
}
fn main() {
let mut db = DatabaseConnection::new();
// The transaction `tx` mutably borrows `db`.
let mut tx = Transaction::new(&mut db);
tx.query("SELECT * FROM users");
// This won't compile because `db` is already mutably borrowed.
let results = db.results(); // ❌🔨
// The borrow of `db` ends when `tx` is consumed by `commit`.
tx.commit();
// Now it is possible to borrow `db` again.
let results = db.results();
}
src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/phantomdata.md
Show resolved
Hide resolved
| - Motivation: We want to be able to "tag" structures with different type | ||
| parameters as a way to tell them apart or pass on lifetime information to | ||
| them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer that we start with one clear motivation. So tagged types sounds good. But why are we tagging them? Could make the example even more concrete?
Here's my suggestion. Say that we have an app with multiple newtype'd integers.
struct UserId(u64);
impl fmt::Display for UserId { ... }
impl FromStr for UserId { ... }
struct PostId(u64);
impl fmt::Display for UserId { ... }
impl FromStr for UserId { ... }
// Pretend we have 100s of newtypes like that and we don't like code duplication.This works, but we have to repeat impls for every newtype.
Next slide, our attempt at implementing a generic newtype'd integer for our app.
struct TaggedIntegerId<T> {
value: u64,
}
struct UserTag;
type UserId = TaggedIntegerId<UserTag>;
struct PostTag;
type PostId = TaggedIntegerId<PostTag>;This is cool, but we get error[E0392]: type parameter T is never used.
From here you can connect to the narrative you already wrote.
WDYT?
(I'm not commenting on the rest of the slide because I'm expecting a major restructuring because of my suggestion.)
| user. `Stage<Start>` offers a lot more contextual detail than `StageStart`, | ||
| and implies there is shared behavior between stages. | ||
|
|
||
| - Demonstrate: We can also capture lifetime parameters with `PhantomData` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we split this onto a separate slide and also provide a more concrete motivation?
I think this is also a better example than the abstract one currently in lifetime-connections.md.
For example:
/// Direct FFI to a database library in C.
/// We got this API as is, we have no influence over it.
mod ffi {
use DatabaseHandle = u8; // maximum 255 databases open at the same time
fn database_open(name: *const c_char) -> DatabaseHandle;
// ... etc.
}
struct DatabaseConnection(DatabaseHandle);
struct Transaction<'a>(&mut DatabaseConnection);
impl DatabaseConnection {
fn new_transaction<'a>(&'a mut self) -> Transaction<'a> {
Transaction(&mut self)
}
}While the Transaction exists, nothing else in Rust can disturb the database, good. But Transaction contains a reference to the database - 8 bytes on a 64-bit platform, whereas the database handle is just 1 byte. That's wasted memory, and an extra indirection. Why don't we simply copy the handle into the transaction? That would save 7 bytes, and avoid jumping through a pointer to access the integer handle value.
struct DatabaseConnection(DatabaseHandle);
struct Transaction<'a>(DatabaseHandle);Oh, that does not compile. PhantomData to the rescue.
| ```rust,editable,compile_fail | ||
| use std::marker::PhantomData; | ||
|
|
||
| pub struct BorrowedButOwned<'a> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example (StringOrigin, BorrowedButOwned) is abstract and a bit confusing. It's not immediately clear what real-world problem this code could solve. The names are generic, and the act of cloning a string to create a "borrowed" value feels counter-intuitive without a concrete motivation (even though that's exactly what would happen in a real-world application!)
I suggest to rewrite this slide to be the motivation and explanation of using PhantomData with generic lifetime parameters (base it upon my suggested struct Transaction(DatabaseHandle) example). Leave the phantomdata.md slide to be purely about using PhantomData with generic type parameters.
|
@tall-vase Thank you for the latest round of improvements! I've just completed a review, and my main feedback is focused on strengthening the narrative flow and making the examples more concrete and motivating for students. The core ideas are excellent, but we can improve the examples and the overall flow. I have tried to leave detailed, actionable comments, used suggested edits and provide code for new less abstract examples. |
Co-authored-by: Dmitri Gribenko <[email protected]>
Co-authored-by: Dmitri Gribenko <[email protected]>
Adds materials on the "leveraging the type system/borrow checker invariants" subject.
I'm still calibrating what's expected subject-and-style wise, so do spell out things where I've drifted off mark.